-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Smooth Native Multi Block Selection #16835
Conversation
This would make this #16811 impossible to achieve right? |
I guess in theory, you can have multiple ranges? |
@youknowriad Not necessarily, since for non consecutive selection, you wouldn't use mouse dragging. But it is true that we cannot set a native selection in that case, or at least not for all of the selection pieces. We'd have to use our own colours... That may look inconsistent though... Cc @jasmussen |
In the case of non consecutive selection, we could maybe only rely on a border/shadow/outline to visualise the selection. |
Here's what I see: Immediate reaction: this is really nice. It solves the issues from #15640 very elegantly, and it even feels very native. It even works nicely with the more exotic blocks: I definitely think we should move forward with this.
Outside of whatever technical magic I'm sure Ella can wield, this can be solved with design. Here's how a selected text block looks in Figma: Here's how a selected text block with the text inside also selected looks: Figma is obviously not a 1:1 with our editor. But we can take some inspiration. In this case, I think it could be solved by keeping the text selection indicator as we have here, but also adding an additional indicator of block selection. It could just be a simple border around a selected block, like Figma, and could look something like this (very quick and dirty mockup, not final suggestion): Question: in addition to showing the selection range as this PR has already accomplished, is it possible to also show a highlight border around the selected blocks? If it is, then this would solve the non-consecutive block selection challenge as well, even if we can't show multiple selection ranges. |
Yes, sure, we can style the selected block however we want. I would recommend not using a background, but only a border, shadow, or outline, ideally in the |
Yes indeed. Shadows are removed in Windows High Contrast mode, so if we have to go with those, we need something else for that mode also -- a transparent outline is a neat hack that works because the transparency becomes opaque in that mode.
Oh if only CSS2's I think that we should choose a color here, in the blue spectrum for the default theme and potentially themed for the others. Reason being that a shade of blue is the default selection color for most operating systems, and likely to be subconsciously connected to "selections" for that reason.
I suppose "currentColor" would solve that because presumably the themer who chose a black background would also choose light text. But it also probably wouldn't be bulletproof because a random block might have a different You mention ...
We could look into https://www.w3schools.com/cssref/pr_mix-blend-mode.asp for that, potentially? |
This comment has been minimized.
This comment has been minimized.
02af322
to
def6860
Compare
@jasmussen Would love if you could test this again. I made some changes which should make selection now really smooth. |
Quick GIF: This feels really responsive to me. While there are some little things could could use a little TLC, like the select style and the lingering hover styles, overall this feels like a superbly solid direction. I think this can totally be made to work, impressive! I noticed that when you select downwards across an image, it's "highlighted", but not when you select upwards: — until you release the mouse of course and then it's selected. One of the greatest benefits of going with the native selection style, is that you get the "grayed selection" when the window loses focus. This works, but it doesn't work when the link dialog shows: I could swear that worked in the past attempt that simply removed the colors. Do you know what this might be? And finally — this feels like it can be the first step towards the ability to select between the middles of two paragraphs to merge them. Do you think we can enable that in a future PR? :) |
Yeah, I don't know what's wrong with the hover styles. Need to have a look into it. The image issue would be a native thing. Which browser are you using? It might be that you need to move a little further to select it. Selection is completely in the hands of the browser here, there's not much I can do.
Yes, totally. This is also meant as a step towards partial block selection. |
I'll jump in and help as soon as I can. I'm working on some jet lag at the moment. In sure Monday I'll be alright. All sounds good. Curious about the image thing, selecting from after the image and upwards, I'll test a little more. I'm using chrome. Did you have any thoughts on the inactive select style for links? |
@jasmussen Fixed the hover styles. I have a hard time understanding what you mean from the GIF. Could you show are explain a little more? What's different from master? |
This comment has been minimized.
This comment has been minimized.
Fixed the Safari issue. Should make things smoother in general too in other browsers. |
Nothing's different from master, so to clarify there's nothing you have to fix here at all. But I was hoping that moving from colored selections to stock native selections would fix a lingering issue I have with our link box:
My hope was that the unfocused selection would be present when the link dialog is open, as opposed to just just floating in space, and I could swear I had seen this being the case. But I just spun up #15640 to verify, and no — that link box still floats in space, sadly. So nevermind my comment — I'd still love for us to fix the link issue, but it's definitely a separate issue from this PR, and will probably be easier to fix when this is merged. So continue blazing a trail, I'm super excited about this branch (and can confirm the hover issues are fixed). |
3dd9ab1
to
cc709da
Compare
@youknowriad @jasmussen So far I can think of only one way to have a native selection (colour) over the entire block. It's a bit hacky, but we could insert a transparent image (resized to fit the block) and select that, instead of all the block contents. What is selected doesn't matter anyway, it's purely visual. We overwrite the contents of the clipboard on copy/cut. |
@youknowriad Here's what it looks like now. |
Nice trick. I think the fact that we still see the text selection happening in the background defeats the purpose of the trick a little bit. I'm ok moving forward without the trick. Thoughts? |
@youknowriad Yeah, I think I agree. We will anyway have to remove it again when we implement partial black selection. I’ll fix up this PR in a bit. |
In the worst case we can always revert the selection colour but keep the smooth selection. |
7a9ce87 (HEAD -> try/native-multi-select, origin/try/native-multi-select) Fix selection in Firefox eb668d3 Fix bad rebase 71bf361 Fix block deletion e2e test 386810a Add mouse drag test 41cb592 Rewrite tests c22a36b Polish 9832732 Remove mover min-height and multi select top border cac2c20 Fix cross selection delay in Safari 116e89f Hide hover effect on mouseleave a3f1547 Correct toolbar position 24f817d contenteditable false when there is multi selection 4eade53 Fix selection setting 9182fd3 Remove elements in the way of selection 98dae74 Reselect block correctly f1b08ac Smooth selection 3fb633a Native Multi Block Selection
fe9aca6
to
b46ad9c
Compare
def7417
to
9ea4bc4
Compare
@youknowriad I addressed the feedback. Would be grateful for another pass. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely convinced that this selection modal is better than master from the user's perspective but let's try it.
I'm convinced that the selecting feels much smoother and better, but I'm not 100% convinced that the visual block selection is clear. This last part is easy to adjust later though. We can always re-add the old selection colour if necessary, or explore different solutions. Thank you @youknowriad and @jasmussen for the review. Needless to say, if any issues arise, I'll quickly follow up. |
I'm out for the weekend but I'm confident in this selection and that we can refine it further, and I plan to look at it Monday. |
@jasmussen Don't look at your work emails. 😄 |
* Limit additional selection marker to top level blocks. Addresses #16835 (comment). * Provide exception for placeholderes.
@ZebulanStanphill The extra lines should be gone now. |
Description
I had to disable
contenteditable
for multi-selection, because browsers won't allow selection across editable hosts. This actually works quite well.How has this been tested?
Select multiple block by holding and releasing a mouse press, just like you would for normal selection.
Screenshots
Types of changes
Bug fix.
Checklist: